-
Notifications
You must be signed in to change notification settings - Fork 747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Support non-string parameters. Closes #1236 #2317
Conversation
Addresses #1236 |
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: AdamKorcz <adam@adalogics.com> Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
pkg/apis/sensor/v1alpha1/types.go
Outdated
// When true, a number, boolean, json or string parameter may be extracted. When the field is unspecified, or explicitly | ||
// false, the behavior is to turn the extracted field into a string or json. (e.g. when set to true, the parameter | ||
// 123 will resolve to the numerical type, but when false, or not provided, the string "123" will be resolved) | ||
UseRawDataValue bool `json:"useRawDataValue,omitempty" protobuf:"bytes,5,opt,name=useRawDataValue"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Please add it to the end of the struct - do not change the order of original properties.
-
If it not specified, consider it as the
string
(reverting fix: payload serialization in sensor. Fixes #2272 #2273), in this case, if expecting a JSON, just set it totrue
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 made the change. let me know if the wording of this comment can be improved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe making it a bit simpler like UserRawData
?
Also, it would be better to add this to the doc.
https://github.com/argoproj/argo-events/blob/master/docs/tutorials/02-parameterization.md
All the rest looks good to me! Thanks again!
…nal in documentation Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Signed-off-by: Aalok <aalok_ahluwalia@intuit.com>
Checklist: